Skip to content

feat(otel): Parent Workflow span onto shared execution trace - #647

Draft
ayushiahjolia wants to merge 1 commit into
mainfrom
feat/otel-backend-parented-topology
Draft

feat(otel): Parent Workflow span onto shared execution trace#647
ayushiahjolia wants to merge 1 commit into
mainfrom
feat/otel-backend-parented-topology

Conversation

@ayushiahjolia

Copy link
Copy Markdown
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue Link, if available

#644

Description

Anchor every durable execution on one trace: the propagated remote server span is the execution ancestor when valid, else a deterministic synthetic root. Workflow and Invocation spans share the trace; sampling follows the header (explicit Sampled preserved, unset deferred to the configured sampler).

Also keep the local test runner's execution ARN stable across reinvocations to match the backend, and pin the OTel conformance workflow to the backend-parented topology assertions.

Demo/Screenshots

N/A

Checklist

  • I have filled out every section of the PR template
  • I have thoroughly tested this change

Testing

Unit Tests

Have unit tests been written for these changes? Updated.

Integration Tests

Have integration tests been written for these changes? Added/Updated

Examples

Has a new example been added for the change? (if applicable) N/A

@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 24, 2026 20:05 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 24, 2026 20:05 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the feat/otel-backend-parented-topology branch from 5c0f1a8 to 95fb7cd Compare August 24, 2026 22:15
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 24, 2026 22:15 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 24, 2026 22:15 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the feat/otel-backend-parented-topology branch from 95fb7cd to d97ef9c Compare August 24, 2026 23:02
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 24, 2026 23:03 — with GitHub Actions Inactive
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 24, 2026 23:03 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the feat/otel-backend-parented-topology branch from d97ef9c to 6896e3d Compare August 25, 2026 18:08
@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the feat/otel-backend-parented-topology branch from d226218 to c0df70f Compare August 26, 2026 00:25
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 26, 2026 00:25 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the feat/otel-backend-parented-topology branch from c0df70f to 0ff8de7 Compare August 26, 2026 06:06
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 26, 2026 06:06 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@ayushiahjolia
ayushiahjolia force-pushed the feat/otel-backend-parented-topology branch from 0ff8de7 to 8a88e75 Compare August 26, 2026 06:23
@ayushiahjolia
ayushiahjolia deployed to ai-pr-review-runtime August 26, 2026 06:40 — with GitHub Actions Active
@ayushiahjolia
ayushiahjolia temporarily deployed to ai-pr-review-runtime August 26, 2026 06:40 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Found three issues affecting sampling policy, RECORD_ONLY propagation, and public API compatibility. Static inspection only, as requested.

Reviewed commit 8a88e75b8b72855f5aa3ab9ff6bea9888b3af08b. Workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Claude AI review

Review: feat(otel) — Parent Workflow span onto shared execution trace (#647)

Reviewed the full SHA-anchored diff (27 files, +2473/−270) against the base revision, focusing on the trace-topology change, sampling precedence, cross-classloader bridging, checkpoint/replay semantics, and the local test runner.

No blocking issues found. The change is coherent and unusually well-tested. Key correctness properties I verified:

  • Trace stability across reinvocations. ExecutionTraceContext.canonicalTraceId/resolve derive a stable ancestor (valid remote parent → else ARN/start-time synthetic root) and deliberately never adopt the per-invocation ambient span, so the execution keeps one trace ID. LocalDurableTestRunner now pins executionName/executionOperationId and the ARN's last segment across run() calls, matching that invariant.
  • Sampling is resolved once and applied consistently. resolveSamplingResult follows explicit upstream Sampled → same-trace ambient bit → configured sampler (once) → documented default-sampled; DurableSampler returns the precomputed SamplingResult verbatim (preserving RECORD_ONLY) and the execution ancestor's flags are derived from the same decision, so a parent-based sampler can't silently drop the tree.
  • Cross-classloader decision bridge is scoped correctly. The thread-ID-keyed system property is opened/closed synchronously around each startSpan() (sampler runs on the same thread), with prior-value restore and test cleanup — no leak or cross-thread bleed.
  • Replay/large-result path is safe. The stored EXECUTION operation shares the fresh op's stable id, so the merge collapses to one op (guarded against >1) with no duplicate-key collision; covered by the new 7MB replay test. Continuation/replay operation spans link back to the deterministic initial op id in [operation, Workflow] order.
  • Validity hardening. ExtractedContext tri-state sampling with null→UNDECIDED normalization (incl. Jackson legacy deserialization), and TraceId.isValid/SpanId.isValid rejection of all-zero Root/Parent, are correctly threaded through resolution.

Residual risk (not defects): The nullable-traceId branch in ExecutionOtelPlugin.resolveParentContext is currently unreachable only because executionTrace is set before tracingEnabled and never reset; if that ordering invariant ever changes, SpanContext.create would receive a null trace ID. Consider a defensive fallback to the canonical/derived trace ID there for robustness. Otherwise, verify the OTel conformance workflow (pinned to a new ref in this PR) passes against the backend-parented topology assertions, since that behavior isn't exercised by the in-repo unit/integration suite.

Reviewed commit 8a88e75b8b72855f5aa3ab9ff6bea9888b3af08b. Workflow run

@ayushiahjolia
ayushiahjolia force-pushed the feat/otel-backend-parented-topology branch from 8a88e75 to 40ee121 Compare August 26, 2026 14:12
@ayushiahjolia
ayushiahjolia requested a deployment to ai-pr-review-runtime August 26, 2026 14:12 — with GitHub Actions In progress
@ayushiahjolia
ayushiahjolia requested a deployment to ai-pr-review-runtime August 26, 2026 14:12 — with GitHub Actions In progress
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant